[2.3] Reworked gallery.phtml to move generation of gallery json strings to own block functions by gwharton · Pull Request #18440 · magento/magento2

gwharton · 2018-10-07T17:35:49Z

Description

Reworked gallery.phtml to move generation of gallery json strings to own block functions
Introduced unit tests to cover generation of gallery json strings with full type checking.

The resultant code is much easier to diagnose and test. It uses the json_encode and json_decode routines instead of the old manual way of generating json strings that were used in the old gallery.phtml which is prone to error and mistakes.

Fixed Issues (if relevant)

none. Rework and unit tests.

Manual testing scenarios

View source on any product page.
Ensure the gallery section has fully formed json for gallery and gallery fullscreen sections.

Stock Magento should be

"options": {"nav":"thumbs","loop":true,"keyboard":true,"arrows":true,"allowfullscreen":true,"showCaption":false,"width":700,"thumbwidth":88,"thumbheight":110,"height":700,"transitionduration":500,"transition":"slide","navarrows":true,"navtype":"slides","navdir":"horizontal"}

and

"fullscreen": "nav":"thumbs","loop":true,"navdir":"horizontal","navarrows":false,"navtype":"slides","arrows":true,"showCaption":false,"transitionduration":500,"transition":"slide"}

All strings/bools/ints are now typed properly as per the Magento dev docs for the Magento 2 Gallery Widget.

Contribution checklist

Pull request has a meaningful description of its purpose
All commits are accompanied by meaningful commit messages
All new or changed code is covered with unit/integration tests (if applicable)
All automated tests passed successfully (all builds on Travis CI are green)

gwharton · 2018-10-07T18:48:11Z

The 2.2 Backport PR is here #18443

Changes incorporated

magento-engcom-team · 2019-01-28T16:27:45Z

Hi @sidolov, thank you for the review.
ENGCOM-3987 has been created to process this Pull Request

sidolov · 2019-03-05T17:43:29Z

Hi @gwharton , it's a really good changes that helps external developers customize gallery configuration, but unfortunately, we are not allowed to add new public methods to blocks that marked as @api.
As alternative solution you may extract logic to the separate class, inject it to block via di configuration and use in the phtml template. You can find the example in the /app/code/Magento/Sales/view/adminhtml/templates/order/create/form/address.phtml
Thank you!

gwharton · 2019-03-06T02:16:13Z

@sidolov, Thats neat. I hadn't realised you could do that.

I've updated the 2.3 and 2.2 PR's accordingly. Just waiting on the Travis backlog, but pretty sure they should come in green. If not I will resolve.

magento-cicd2 · 2019-03-06T10:02:27Z

Thank you for your submission, we really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.

… own block functions.

gwharton · 2019-03-18T12:29:51Z

Just waiting on a travis pass.

I have reverted all changes to Gallery.php and GalleryTest.php as all active changes were moved to GalleryOptions.php and GalleryOptionsTest.php and the changes that were left in the original Gallery.php and GalleryTest.php were cleanup code unrelated to this PR and a distraction.

This PR feels much cleaner now.

magento-engcom-team · 2019-03-29T15:09:48Z

Hi @sidolov, thank you for the review.
ENGCOM-3987 has been created to process this Pull Request

magento-engcom-team · 2019-03-30T07:37:39Z

Hi @orlangur, thank you for the review.
ENGCOM-3987 has been created to process this Pull Request

VasylShvorak · 2019-04-03T11:02:28Z

✔️ QA passed

m2-assistant · 2019-04-08T11:38:38Z

Hi @gwharton, thank you for your contribution!
Please, complete Contribution Survey, it will take less than a minute.
Your feedback will help us to improve contribution process.

…ry json strings to own block functions #18440

magento-engcom-team added Area: Frontend Component: Catalog Release Line: 2.3 labels Oct 7, 2018

gwharton mentioned this pull request Oct 7, 2018

[Backport][2.2] Reworked gallery.phtml to move generation of gallery json strings to own block functions #18443

Merged

4 tasks

ihor-sviziev self-assigned this Oct 8, 2018

sidolov previously requested changes Oct 11, 2018

View reviewed changes

Comment thread app/code/Magento/Catalog/Block/Product/View/Gallery.php Outdated

Comment thread app/code/Magento/Catalog/Block/Product/View/Gallery.php Outdated

sidolov added the Progress: needs update label Oct 11, 2018

hostep mentioned this pull request Oct 13, 2018

Fix disappearing navigation arrows in fotorama zoom #18590

Merged

4 tasks

ihor-sviziev removed the Progress: needs update label Oct 16, 2018

ihor-sviziev self-requested a review October 16, 2018 07:50

ihor-sviziev reviewed Oct 17, 2018

View reviewed changes

Comment thread app/code/Magento/Catalog/Block/Product/View/Gallery.php Outdated

ihor-sviziev reviewed Oct 17, 2018

View reviewed changes

Comment thread app/code/Magento/Catalog/Block/Product/View/Gallery.php Outdated

sidolov approved these changes Jan 28, 2019

View reviewed changes

magento-engcom-team assigned sidolov Jan 28, 2019

magento-engcom-team added this to the Release: 2.3.1 milestone Jan 28, 2019

okorshenko removed this from the Release: 2.3.1 milestone Jan 28, 2019

magento-engcom-team added the Progress: accept label Feb 7, 2019

sidolov removed the Progress: accept label Feb 28, 2019

ghost added the Progress: needs update label Mar 5, 2019

ihor-sviziev suggested changes Mar 6, 2019

View reviewed changes

ihor-sviziev self-requested a review March 7, 2019 05:11

ihor-sviziev removed the Progress: needs update label Mar 7, 2019

ihor-sviziev approved these changes Mar 7, 2019

View reviewed changes

ghost added the Progress: ready for testing label Mar 7, 2019

ghost added Progress: needs update and removed Progress: ready for testing labels Mar 16, 2019

Graham Wharton added 2 commits March 18, 2019 12:11

Moved generation of Gallery options and Gallery fullscreen options to…

2ce4ab0

… own block functions.

Removed Unnecessary type checking on tests

dcc71bf

Ignore PHPMD Else Expression warnings

6c0c2fa

sidolov approved these changes Mar 29, 2019

View reviewed changes

ghost added Progress: ready for testing and removed Progress: needs update labels Mar 29, 2019

orlangur approved these changes Mar 30, 2019

View reviewed changes

gwharton mentioned this pull request Apr 3, 2019

Product gallery caption issue #16580

Closed

VasylShvorak self-assigned this Apr 3, 2019

ghost added Progress: testing in progress and removed Progress: ready for testing labels Apr 3, 2019

ghost added Progress: extended testing and removed Progress: testing in progress labels Apr 3, 2019

p-bystritsky self-assigned this Apr 3, 2019

p-bystritsky added 2 commits April 3, 2019 16:27

Merge branch '2.3-develop' into 2.3-develop-gallery-rework

927887f

#18440: Static test fix.

e29fa3d

ghost added Progress: accept and removed Progress: extended testing labels Apr 4, 2019

magento-engcom-team added the Award: special achievement label Apr 5, 2019

magento-engcom-team merged commit e29fa3d into magento:2.3-develop Apr 8, 2019

magento-engcom-team added this to the Release: 2.3.2 milestone Apr 8, 2019

magento-engcom-team pushed a commit that referenced this pull request Apr 8, 2019

ENGCOM-3987: [2.3] Reworked gallery.phtml to move generation of galle…

4f845f2

…ry json strings to own block functions #18440

gwharton deleted the 2.3-develop-gallery-rework branch April 23, 2019 20:21

Uh oh!

Conversation

gwharton commented Oct 7, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Fixed Issues (if relevant)

Manual testing scenarios

Contribution checklist

Uh oh!

gwharton commented Oct 7, 2018

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

magento-engcom-team commented Jan 28, 2019

Uh oh!

sidolov commented Mar 5, 2019

Uh oh!

gwharton commented Mar 6, 2019

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

magento-cicd2 commented Mar 6, 2019

Uh oh!

gwharton commented Mar 18, 2019

Uh oh!

magento-engcom-team commented Mar 29, 2019

Uh oh!

magento-engcom-team commented Mar 30, 2019

Uh oh!

VasylShvorak commented Apr 3, 2019

Uh oh!

m2-assistant Bot commented Apr 8, 2019

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

9 participants

gwharton commented Oct 7, 2018 •

edited

Loading